-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix/76667 - Contact Import button not clickable due to autoscroll resets the list #78889
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@Krishna2323 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Will review tomorrow. |
|
@Expensify/design we now have the import button in the fixed footer, could you please check if that looks correct or not?
@dmkt9 Have you checked whether there’s a possibility of showing multiple actions in the fixed footer? That would look bad, so we need to ensure that only a single action appears in the fixed footer. |
🤔 Hmm, is this what we want though? Feels like it would make more sense to me to just have it as the first item of the list beneath the search input. @dubielzyk-expensify what do you think? |
Very much with you, Danny. This feels wrong to me. I'd expect it to be the first item in the list and it can be scrolled away like anything else. |
|
Thanks for the confirmation. @dmkt9 ^ |
|
@dmkt9 bump! |
|
@dannymcclain @dubielzyk-expensify Hi, does the result below look good to you? 2026-01-15.18-40-39.mp4 |
|
Placement looks great, but I think it should scroll away just like all the other options. |
|
Agree with scrolling the option away as well 👍 Also what happen to the fonts, shouldn't some of them be bold? |
|
@dannymcclain @dubielzyk-expensify Thank you for the confirmation. Here is the current result: 2026-01-16.09-11-29.mp4 |
|
@Krishna2323 Thanks for waiting. It’s ready for review again. |
|
That looks more right to me, but why isn't the fonts bold? Is this a system thing? |
@dubielzyk-expensify Yes. It’s bold by default. The normal font weight is shown below:
|
Yes. I mean we are already displaying it in bold. And the bold style might be hard to notice on my device, possibly due to the font on my Android. |
|
@dubielzyk-expensify Ah, sure. I sent that image to illustrate that in the video, the text is still bold, and that’s also our latest result:
|
|
Perfect, thank you! Let's see what @dannymcclain thinks then we can move into code review and do a test build |
|
Looks great to me. Let's keep this one moving! 🚀 |





Explanation of Change
Fixed Issues
$ #76667
PROPOSAL: #76667 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
android.native.mp4
Android: mWeb Chrome
android.mweb.mp4
iOS: Native
ios.native.mp4
iOS: mWeb Safari
ios.mweb.mp4
MacOS: Chrome / Safari
mac.mweb.mp4